Skip to content

Replace forked processes that fail to boot - #760

Merged
rosa merged 3 commits into
rails:mainfrom
sapandiwakar:fix/751-recover-unready-forks
Jul 29, 2026
Merged

Replace forked processes that fail to boot#760
rosa merged 3 commits into
rails:mainfrom
sapandiwakar:fix/751-recover-unready-forks

Conversation

@sapandiwakar

Copy link
Copy Markdown
Contributor

What

Track each forked process until its boot callbacks finish. If a child does not report readiness within a configurable startup timeout, terminate it and let the existing reap-and-replacement path replace that configured process.

This applies only to fork mode. The default startup timeout is five minutes.

Why

ForkSupervisor currently replaces children only when waitpid2(..., WNOHANG) reports that they exited. A child that remains alive while blocked during boot or registration is never returned by waitpid2, so the supervisor can retain an unusable child indefinitely without replacing it.

For a dispatcher, this means the process can remain present while never reaching its polling loop. Due scheduled jobs then remain in solid_queue_scheduled_executions instead of being moved to ready executions.

This addresses the concrete recovery gap observed in #751. It does not attempt to detect a process that successfully booted and later stopped making progress.

Reproduction

A deterministic local reproduction is to block a dispatcher in an existing startup hook:

SolidQueue::Dispatcher.on_start { sleep 60 }

Start Solid Queue in fork mode with a dispatcher configured.

On main, the dispatcher child remains alive but never reaches its polling loop. Because it has not exited, waitpid2(..., WNOHANG) does not return its PID and the supervisor never replaces it.

On this branch, the behavior can be observed quickly by temporarily lowering the timeout:

SolidQueue.process_startup_timeout = 2.seconds
SolidQueue::Dispatcher.on_start { sleep 60 }

The supervisor logs the startup timeout, terminates that dispatcher PID, and starts its replacement. The replacement also times out while the hook remains blocked, demonstrating that each configured slot is continuously supervised.

The regression test uses the same failure shape by blocking a worker during registration. It verifies that the stalled child is terminated and replaced while a healthy sibling keeps the same PID.

How

  • Create a readiness pipe for each fork.
  • Have the child signal readiness after all boot callbacks complete.
  • Track each unready PID using a monotonic startup timestamp.
  • Terminate only the PID that exceeds the timeout, then reuse the existing replacement path.
  • Close readiness descriptors during successful startup, replacement, and shutdown.
  • Emit a warning containing the affected child PID.

A timed-out child is killed rather than gracefully terminated because a process blocked during boot cannot reach its run loop to complete a graceful stop.

Tests

  • A fork blocked during registration is terminated and replaced.
  • A healthy sibling is not restarted when another child times out.
  • A child that becomes ready before the deadline survives a complete supervision cycle.
  • Startup timeout warnings include the actual child PID.

Relates to #751.

@sapandiwakar
sapandiwakar force-pushed the fix/751-recover-unready-forks branch from ef70db9 to b5aee2b Compare July 20, 2026 08:06
@sapandiwakar
sapandiwakar marked this pull request as ready for review July 20, 2026 08:20
@rosa
rosa force-pushed the fix/751-recover-unready-forks branch from b5aee2b to b99502f Compare July 27, 2026 17:12
@sapandiwakar

Copy link
Copy Markdown
Contributor Author

Hi @rosa 👋 Thanks for rebasing this onto current main!

I looked into the three failing jobs and I'm fairly confident they're pre-existing flakes rather than something introduced by this patch:

  • Both SQLite failures (Ruby 3.2 and 4, Rails 7.2) start in SolidQueue::JobTest#test_try_to_discard_claimed_job and then cascade into SQLite3::BusyException errors. The rebase's parent commit fails the same way without this patch: https://github.com/rails/solid_queue/actions/runs/30112624739
  • The MySQL failure (Ruby 3.3, Rails 8.0) is the recurring-task instrumentation timing test you mentioned in Flaky tests #602 as the flakiest one in the suite.

The previous head of this branch was green across all 64 checks with the same substantive change. A rerun of the three jobs should clear them — I don't have permissions to retrigger them myself.

On the approach itself: happy to discuss if you'd prefer a different shape for this, e.g. a smaller supervisor-side contract instead of the readiness pipe. Just let me know and I can adjust the scope.

@rosa

rosa commented Jul 28, 2026

Copy link
Copy Markdown
Member

Hey @sapandiwakar, yes! These failures are flaky tests, nothing to do with your changes.

Nice catch and fix on this bug! I'm making some changes to the solution, mostly around how the callback works. It doesn't seem right to pass over a callback named on_fork_ready around to the generic Supervisor class and down to the processes.

I tried to keep AsyncSupervisor and ForkSupervisor as minimal as possible, with the common part living in Supervisor, but this change makes ForkSupervisor diverge from that.

I'll push some changes to see what you think.

@rosa

rosa commented Jul 28, 2026

Copy link
Copy Markdown
Member

Ok, I've pushed 3 commits with some changes. I've renamed process_startup_timeout to fork_startup_timeout, because it applies only to forks (not even to the supervisor, because it doesn't have a parent that can monitor it), whereas the other configuration options, process_heartbeat_interval and process_alive_threshold apply to all processes, so I thought it was good to distinguish that. It's an option that doesn't do anything in async mode.

Then, I've refactored the code considerably, leaving the Supervisor class as it was before, and hiding the startup checks in the processes themselves and the ForkSupervisor only.

@rosa

rosa commented Jul 28, 2026

Copy link
Copy Markdown
Member

I might still do some smaller changes. The config option is the only public change so that one needs to stay when merging, but the rest can change.

@rosa

rosa commented Jul 28, 2026

Copy link
Copy Markdown
Member

Going to test this in our app in production to make sure there aren't any issues we might have missed.

@rosa
rosa force-pushed the fix/751-recover-unready-forks branch from c32d091 to f616adf Compare July 29, 2026 09:34
rosa and others added 2 commits July 29, 2026 11:48
The tests killed the whole process group to clean up, but the
supervisor under test is actively replacing its stalled forks, and a
group KILL races that forking: a fork spawned concurrently with the
kill can escape it, wake up from its stalled boot long after the test
has finished, register itself and start polling, holding the test
database's write lock and cascading lock timeouts through unrelated
tests, as seen on CI with SQLite.

Terminating the supervisor gracefully instead lets it clean up its own
forks: it stops replacing them first, and a fork stuck in boot is
covered by the QUIT escalation, as its signal handlers are installed
before the boot callbacks where it stalls. This also lets go of the
replacement fork's liveness assertion: the replacement stalls too, so
whether it's alive depends on where its own boot timeout is.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the startup-tracking machinery out of ForkSupervisor into boot
guards with a common interface, created by each process when it
starts, according to how it runs: a forked process pipes its boot
completion to the supervisor through a BootGuards::ForkGuard, created
before forking so it survives it, while processes running as threads
of their supervisor, or inline, use a BootGuards::NullGuard, which
completes on boot without any monitoring. Booting itself completes
the guard, so starting a process remains just boot and run.

ForkSupervisor checks its forks' boot guards while checking for
terminated processes to replace, terminating any fork whose boot has
timed out so that it's reaped and replaced as usual, and closing each
guard together with the rest of the fork's cleanup, asking the process
instances themselves, like the async supervisor does when it checks
whether they're alive.

The configurable timeout becomes fork_boot_timeout, as it only applies
to forked processes: supervised processes running as threads aren't
monitored while booting, and the supervisor itself has no parent
within Solid Queue that could enforce a timeout on it. The
instrumentation event follows suit. Also reuse Timer's monotonic clock
instead of redefining it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa
rosa force-pushed the fix/751-recover-unready-forks branch from f616adf to 3504c4e Compare July 29, 2026 09:48
@rosa
rosa merged commit f8899df into rails:main Jul 29, 2026
115 of 116 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants